feat(desktop): share the Local Runtime Host - #3972
Conversation
d974723 to
b452e05
Compare
jackwener
left a comment
There was a problem hiding this comment.
I re-reviewed the current exact head 6f57620a3f11f42f47e06a901bd52ceb7565e2d6. The recovery changes close the previous retry lock, but the PR is still not ready to approve because the owner-credential lifecycle remains incomplete.
Findings
P1 — Claimed connection codes accumulate durable full-owner credentials without a local revocation path.
Both initial setup and every later connection code create a fresh desktop-owner:<uuid> principal (apps/desktop/src/main/runtime-host-local-remote-access.ts:251 and :573). Credential finalization revokes existing active credentials only when their principal matches (packages/runtime-host/src/server/access-authority.ts:361-366), so these random principals never replace one another.
I exercised the current access-authority path by preparing and finalizing two codes. The durable access file retained two distinct active Remote Owner credentials, each bound to a different Client instance. These owners receive the complete REMOTE_OWNER_OPERATION_GRANTS, but Local Settings exposes only “New connection code”, “Turn off”, and “Remove background service” (runtime-host-profiles-section.tsx:391-405); it has no authorization list or per-client revoke action. Turning off Direct peer access or uninstalling the service also does not revoke credentials stored in the State Root.
Before merge, either make this one stable Local-share owner slot that can be replaced and revoked, or explicitly support multiple owners with understandable device labels, an authorization/status list, and individual revocation.
P2 — Connection-code failures are not actionable in English.
The import dialog sends every failure through settingsActionErrorMessage() (runtime-host-connection-code-dialog.tsx:52-61). For English, that helper does not classify the error and then returns unknownError even after safely redacting the original message (settings-error-copy.ts:31-35). A user therefore cannot distinguish malformed, expired, already-claimed, unreachable, or root-mismatch failures. Preserve typed failure reasons across the bridge and map them to safe, specific recovery guidance.
P2 — The UI does not explain the security and connectivity contract.
The current copy says only that the feature uses an experimental Direct peer and that the code is one-time. It should also state that there is no fallback transport, an unclaimed code expires after 15 minutes, the first claimant receives full Owner authority, and turning off Direct access or removing the service does not revoke an already claimed credential.
Verification
The new persisted handoff, peer-change, uninstall, and cleanup intents now recover through startup or the enabled recovery action, so the previous unavailable UI dead end is closed. Affected builds and 121 focused Desktop, CLI, and Runtime Host tests passed. All applicable hosted checks are successful; the conditional Eval job is skipped. The merge result against current main (45824cc78eaf0e02bbd07dbd6fd46adfd38bd985) is clean.
Posted by an automated review agent operated by @WAWQAQ. This is not an
independent human review and does not satisfy the committer review required by
CONTRIBUTING.md. A human is accountable for this comment — please push back if
anything here is wrong.
简体中文
本条评论由 @WAWQAQ 运行的自动化审查程序发出。它不构成 CONTRIBUTING.md
所要求的独立人类审查,也不能替代人类审查。有人类对本条评论负责,如有错误请直接指出。
6f57620 to
491efd4
Compare
EnglishAddressed the confirmed findings at
The branch was rebased onto current 简体中文已在
分支已 rebase 到最新 |
jackwener
left a comment
There was a problem hiding this comment.
I re-reviewed the current exact head 491efd4ed4b71e6a09b09ce79bd773d04c38c032. The previous owner-lifecycle and connection-code UX findings are addressed, but one revocation race still makes this unsafe to approve.
P1 — “Revoke shared access” can report success while a newly claimed Owner remains active.
The Desktop implementation first lists credentials and selects the currently active credential, then revokes that credential by ID (apps/desktop/src/main/runtime-host-local-remote-access.ts:314-336). These are separate Host operations.
If an outstanding connection code is claimed between those operations, credential finalization removes the listed active credential and activates the new same-principal credential (packages/runtime-host/src/server/access-authority.ts:338-380). The later revoke of the stale ID returns { revoked: false } (access-authority.ts:300-305), but the Desktop ignores that result, clears its sharedAccess snapshot flag, and displays “Shared access revoked.” The newly claimed credential remains a durable Remote Owner with the full Owner grants.
I reproduced this with the current production access-authority implementation: after finalizing the pending successor and revoking the previously listed credential ID, the revoke result was false and the durable access file still contained the successor as active, bound to the new Client instance. The added Desktop test always mocks { revoked: true }, so it does not cover finalization racing between list and revoke.
Please make revocation a single Host-owned atomic revoke-by-principal operation. If that is not practical, the caller must at least inspect the revoke result and use bounded re-list/retry and postcondition verification until no matching active or pending credential remains; only then should the UI report success.
Posted by an automated review agent operated by @WAWQAQ. This is not an
independent human review and does not satisfy the committer review required by
CONTRIBUTING.md. A human is accountable for this comment — please push back if
anything here is wrong.
简体中文
本条评论由 @WAWQAQ 运行的自动化审查程序发出。它不构成 CONTRIBUTING.md
所要求的独立人类审查,也不能替代人类审查。有人类对本条评论负责,如有错误请直接指出。
Move the Local Host to its existing managed-service lifecycle when remote access is enabled, while retaining Local IPC and the same State Root. Add one-time Direct peer connection codes and Desktop import/export UX without creating a second Host identity. Generated-by: OpenAI Codex
Bind a pairing candidate to the first Desktop that finalizes it, and reconnect that Client onto the resulting Owner authority. Recover interrupted managed-service handoffs from a durable journal and close operator lifecycle races while consolidating framed child handling. Generated-by: OpenAI Codex
Persist exact peer and uninstall intents so interrupted Desktop operations resume safely. Preserve credential reload invariants and explicit active-work interruption authority. Generated-by: OpenAI Codex
Make managed setup, peer changes, and uninstall converge on exact persisted targets across interruption boundaries. Revalidate pairing and peer authority before admitting a connection. Generated-by: OpenAI Codex
Use one replaceable Local sharing principal and expose explicit revocation independently from connectivity and service lifecycle. Preserve bounded connection-code failure reasons for localized recovery guidance.
Serialize principal-wide revocation with pairing finalization so a replacement credential cannot survive stale credential selection. Have Desktop invoke the local-owner-only operation directly. Generated-by: OpenAI Codex
491efd4 to
7e3411b
Compare
EnglishThe race is valid and is fixed at Revocation is now the Local Owner-only
Desktop now calls this operation directly instead of listing credentials and revoking a stale ID. The operation is deliberately not granted to Remote Owners. A focused authority test covers both orderings and verifies that neither credential remains authenticatable; the Desktop test verifies the principal-scoped request. The branch is rebased onto current 简体中文该竞态成立,已在 撤销现在由仅限 Local Owner 的 Host 操作
Desktop 不再先列举 credential 再按可能过时的 ID 撤销,而是直接调用 principal 级操作。该操作没有授予 Remote Owner。定向 authority 测试覆盖了两种顺序并验证两份 credential 最终均无法认证;Desktop 测试验证了 principal-scoped 请求。 分支已 rebase 到最新 |
jackwener
left a comment
There was a problem hiding this comment.
I re-reviewed the current exact head 7e3411b4694b01799b443f464ceeff85f39b090e. The previously reported owner-lifecycle, recovery, connection-code UX, security-copy, and revocation-race findings are closed, and I found no remaining P0–P2 issue.
The final change makes shared-access revocation one Host-owned principal operation serialized with pairing finalization. Both possible orderings leave the old and pending/new credentials unable to authenticate, persistence ambiguity still fails closed, the operation is unavailable to Remote Owners, and compatibility epoch 55 prevents mixed-version use. Focused Runtime Host and Desktop verification passed. The branch is directly based on current main, and its merge tree is clean.
The hosted test and quality jobs were still queued when I submitted this review. This approval covers the code review; merging should still wait for all required hosted checks to finish successfully on this exact head.
Posted by an automated review agent operated by @WAWQAQ. This is not an
independent human review and does not satisfy the committer review required by
CONTRIBUTING.md. A human is accountable for this comment — please push back if
anything here is wrong.
简体中文
本条评论由 @WAWQAQ 运行的自动化审查程序发出。它不构成 CONTRIBUTING.md
所要求的独立人类审查,也不能替代人类审查。有人类对本条评论负责,如有错误请直接指出。
Keep the preset boundary assertion synchronized with the new Local Owner-only principal revocation operation. Generated-by: OpenAI Codex
English
Summary
Let a user expose the current computer's Local Runtime Host to another Maka Desktop without creating a second Host or copying its State Root
#3950 is merged; this PR is based on main
Refs #3842
Verification
npm run typechecknpm run lint -- --diagnostic-level=errornpm run format:checknpm run buildnpm run check:stalenpm run check:asf-headersnpm run astryx:surface-inventoryUI
Before:
After:
AI use
Tool(s) and scope: OpenAI Codex assisted with implementation, tests, documentation, and verification under the contributor's direction
Checklist
Does this PR entail a change in behavior?
中文
摘要
允许用户把当前电脑的 Local Runtime Host 提供给另一台 Maka Desktop 使用,同时不创建第二个 Host,也不复制 State Root
#3950 已合并;本 PR 现基于 main
关联 #3842
验证
npm run typechecknpm run lint -- --diagnostic-level=errornpm run format:checknpm run buildnpm run check:stalenpm run check:asf-headersnpm run astryx:surface-inventory界面
修改前:
修改后:
AI 使用
工具与范围:OpenAI Codex 在贡献者指导下协助实现、测试、文档与验证
检查清单
本 PR 是否改变行为?